Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

docs(ngInit): add note on best practices #4366

Closed
wants to merge 1 commit into from

Conversation

btford
Copy link
Contributor

@btford btford commented Oct 10, 2013

Adds a note to the docs about acceptable use cases for ngInit versus when you should just use a controller.

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format
  • Contributor is appropriately sassy

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@petebacondarwin
Copy link
Contributor

Nice one. I never thought of it like that before. I can't think of any other "serious" use of ng-init.

It could be worth mentioning that the ng-init expression executes in the pre-link stage, unlike most directives, which use compile or post-link.

@IgorMinar
Copy link
Contributor

+1

lgtm

@heikki
Copy link

heikki commented Oct 10, 2013

Ha! Nice example for the thing I had to figure a different and not so elegant workaround.

@btford
Copy link
Contributor Author

btford commented Oct 10, 2013

Landed as e819d21.

@gsklee
Copy link
Contributor

gsklee commented Jan 6, 2014

This doesn't feel right. The directive is called ngInit for a reason: it can be used to bootstrap the app with initial data from the server and thus saving HTTP requests, which is a very common optimization practice. While it is true that this directive shouldn't be abused, the new tagline is kinda misleading and making ngInit doing something that's betraying its namesake. It makes merging ngInit into ngRepeat appear to be a very logical next step (as in #5623) but IMHO is totally missing the original point.

I'd suggest to patch the behavior of the directive instead:

  • ngInit can only be used side-by-side with ngApp
  • The value of ngInit can only be a JSON
  • The parsed object will be stored in a special factory (say $init) whose data can be retrieved through DI

For aliasing special property in ngRepeat, make a separate directive or API that's semantically related to ngRepeat.

@caitp
Copy link
Contributor

caitp commented Jan 6, 2014

there's no good reason for any of this --- it can already be used like this, and it doesn't make any sense to add breaking changes just to restrict something to a particular use-case that makes sense for one user.

This is why I don't think the #5623 is an entirely right-minded idea. It might be nice to build aliasing into ngRepeat (although there is certainly an unreasonable cost to this), but it doesn't make sense to entirely replace a perfectly functional tool with many uses, which is certainly heavily used in demos / reproductions, if not production applications.

The idea of restricting something to one particular use-case (which it is already quite capable of serving) does not really add anything particularly helpful, and breaks a lot of things which were already working before

@gsklee
Copy link
Contributor

gsklee commented Jan 6, 2014

@caitp Yep, or we can just find a better way to rephrase these lines.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants